Normalize all LSP URIs #1660
Normalize all LSP URIs #1660
Conversation
There was a problem hiding this comment.
Subclassing the existing TextDocuments service and, e.g., wrapping the Map _syncedDocuments doesn't work (or wouldn't be more elegant) as everything is private or buried within inline function expressions, correct? So I agree to copy-paste the TextDocuments service.
I assume this thing is quite stable in the vscode repo and would only have to be touched in case new document event types would be introduced by the vscode folks, right?
Yes, It'd be great if we could have something like a |
|
I'm however wondering what about langium/packages/langium/src/lsp/language-server.ts Lines 331 to 339 in 99fb880 |
|
@sailingKieler I've mentioned this in the initial post:
Outgoing URIs are no problems, as the language client should parse the malformed URIs correctly - it's just that we cannot deal with "correctly" formatted URIs on the incoming side right now. |
That's an interesting assumption. 😅 I assume that VS Code implementations sitting at the beginning of the message pipeline of our LS (like |
They're not, the they just pass in whatever the language client gives them. They don't perform any manipulation on the LSP data. We do :)
Since I'm running Langium on Windows, I've naturally tested that it still works as expected. Note that this change is a no-op for VS Code, since the URIs we receive from VS Code are the exact same format as the one's we build ourselves using |
msujew
force-pushed
the
msujew/normalize-uris
branch
from
a356895 to
47ae6b8
Compare
|
I think I got it now.
You're basically saying: Everything that happens in https://github.com/microsoft/vscode-languageserver-node/blob/f58f4dff16ad2760028bb1cb95e882de30a1000f/server/src/common/textDocuments.ts#L189-L244 relies on the plain uri strings provided by the language client, while we're doing on first contact with a new document, and later ondocument.uri.toString(), e.g. in langium/packages/langium/src/workspace/documents.ts Lines 409 to 417 in 99fb880 With your changes the book keeping in |
|
@sailingKieler Exactly - and the main issue is essentially this: We try to get the text document based on the langium URI - however, these don't match on Windows for a bunch of language clients. That's why we need to keep every URI formatted in the same way. Note that the latest commit adds a few more utility methods to removes a few hacks inside our test framework. |
Had a look. Great that we can get rid of those workarounds at this occasion. 👌 |
Addresses an issue discussed in #1619.
The main issue is that VS Code itself uses a non spec-compliant URI format in which they encode the
C:toc%A3. This behavior is also mirrored in thevscode-uripackage, which we use to storeLangiumDocumentobjects.However, language clients like LSP4E correctly send
C:- which leads us down a very deep rabbit hole. In the end, the best solution to this is to write our ownTextDocumentsservice that normalizes the incoming URIs to the same schema used by VS Code. This way, any incoming URI can find its respective document in the internal map.Note that it seems like LSP4E can deal with our outgoing URIs just fine - it's the incoming URIs that are problematic.